Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrated code lifecycle: Show ssh fingerprints #9650

Merged
merged 32 commits into from
Dec 20, 2024

Conversation

SimonEntholzer
Copy link
Contributor

@SimonEntholzer SimonEntholzer commented Oct 31, 2024

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) and too complex database calls.
  • I strictly followed the principle of data economy for all database calls.
  • I strictly followed the server coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).
  • I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
  • I documented the Java code using JavaDoc style.

Client

  • Important: I implemented the changes with a very good performance, prevented too many (unnecessary) REST calls and made sure the UI is responsive, even with large data (e.g. using paging).
  • I strictly followed the principle of data economy for all client-server REST calls.
  • I strictly followed the client coding and design guidelines.
  • Following the theming guidelines, I specified colors only in the theming variable files and checked that the changes look consistent in both the light and the dark theme.
  • I added multiple integration tests (Jest) related to the features (with a high test coverage), while following the test guidelines.
  • I added authorities to all new routes and checked the course groups for displaying navigation elements (links, buttons).
  • I documented the TypeScript code using JSDoc style.
  • I added multiple screenshots/screencasts of my UI changes.
  • I translated all newly inserted strings into English and German.

Motivation and Context

Resolves #9527

Description

This PR adds a subpage to the SSH settings, to let users display Artemis key fingerprints

Steps for Testing

  1. Login and navigate to the SSH settings.
  2. Click the fingerprints link
  3. Verify the displayed fingerprint is correct (e.g. delete your known hosts file/remove the test server's entry, and use SSH to clone something)

image

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Client

Class/File Line Coverage Confirmation (assert/expect)
ssh-user-settings-fingerprints.component.ts 100%
ssh-user-settings-fingerprints.service.ts 100%

Server

Class/File Line Coverage Confirmation (assert/expect)
SshFingerprintsProviderService.java 100%
SshFingerprintsProviderResource.java 100%

Screenshots

image
(Shows fingerprints depending on which are configured for the server)
image

Summary by CodeRabbit

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced a service for managing SSH fingerprints.
    • Added a REST controller for retrieving SSH fingerprints via a new API endpoint.
    • Implemented a user interface component for displaying SSH fingerprints in user settings.
    • Enhanced SSH settings with new routes for managing SSH keys and fingerprints.
    • Expanded user settings with new fields and error messages related to SSH key management.
    • Introduced automatic expiration settings for SSH keys.
  • Bug Fixes

    • Improved error handling for SSH key management.
  • Documentation

    • Updated user settings documentation with new error messages and details about SSH keys.
  • Tests

    • Added comprehensive test suites for the new components and services related to SSH fingerprint management.
    • Established tests for the functionality of retrieving SSH fingerprints.

@github-actions github-actions bot added tests server Pull requests that update Java code. (Added Automatically!) client Pull requests that update TypeScript code. (Added Automatically!) database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. core Pull requests that affect the corresponding module programming Pull requests that affect the corresponding module labels Oct 31, 2024
Copy link
Contributor

@iyannsch iyannsch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you like to base this PR on feature/localVC/support-multiple-SSH-keys? In here are most "multi SSH keys" LOCs making it hard to review ☺️

@SimonEntholzer
Copy link
Contributor Author

Would you like to base this PR on feature/localVC/support-multiple-SSH-keys? In here are most "multi SSH keys" LOCs making it hard to review ☺️

Yeah I'm waiting to get #9478 merged, because the finger prints PR depends on the UI changes from that PR, hence it's still draft

@SimonEntholzer SimonEntholzer changed the base branch from develop to feature/localVC/support-multiple-SSH-keys November 12, 2024 09:12
@SimonEntholzer SimonEntholzer changed the base branch from feature/localVC/support-multiple-SSH-keys to develop November 12, 2024 09:18
Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Nov 19, 2024
@SimonEntholzer SimonEntholzer changed the base branch from develop to feature/localVC/support-multiple-SSH-keys November 25, 2024 07:39
@SimonEntholzer SimonEntholzer changed the base branch from feature/localVC/support-multiple-SSH-keys to develop November 25, 2024 07:39
@SimonEntholzer SimonEntholzer changed the base branch from develop to feature/localVC/support-multiple-SSH-keys November 25, 2024 07:49
@SimonEntholzer SimonEntholzer changed the base branch from feature/localVC/support-multiple-SSH-keys to develop November 25, 2024 07:54
@SimonEntholzer SimonEntholzer changed the base branch from develop to feature/localVC/support-multiple-SSH-keys November 25, 2024 07:55
@SimonEntholzer SimonEntholzer force-pushed the feature/ssh/show-ssh-fingerprints branch from 360aefe to b6ef05c Compare November 25, 2024 08:00
@github-actions github-actions bot removed database Pull requests that update the database. (Added Automatically!). Require a CRITICAL deployment. core Pull requests that affect the corresponding module labels Nov 25, 2024
@SimonEntholzer SimonEntholzer marked this pull request as ready for review November 25, 2024 08:38
@SimonEntholzer SimonEntholzer requested a review from a team as a code owner November 25, 2024 08:38
Copy link

⚠️ Unable to deploy to test servers ⚠️

The docker build needs to run through before deploying.

@github-actions github-actions bot added the deployment-error Added by deployment workflows if an error occured label Nov 26, 2024
@SimonEntholzer SimonEntholzer changed the base branch from feature/localVC/support-multiple-SSH-keys to develop November 26, 2024 13:12
Copy link
Contributor

@muradium muradium left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on TS6, fingerprint is displayed correctly

Copy link
Contributor

@raffifasaro raffifasaro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-approve

Copy link
Contributor

@magkue magkue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reapprove

Copy link

@HawKhiem HawKhiem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested on TS3. Reapprove

image

@krusche krusche changed the title Integrated code lifecycle: Show SSH fingerprints Integrated code lifecycle: Show ssh fingerprints Dec 20, 2024
@krusche krusche added this to the 7.8.0 milestone Dec 20, 2024
@krusche krusche merged commit ab8bada into develop Dec 20, 2024
60 of 62 checks passed
@krusche krusche deleted the feature/ssh/show-ssh-fingerprints branch December 20, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Pull requests that update TypeScript code. (Added Automatically!) programming Pull requests that affect the corresponding module ready for review ready to merge server Pull requests that update Java code. (Added Automatically!) tests
Projects
Status: Merged
Status: Done
Development

Successfully merging this pull request may close these issues.

Show SSH fingerprint